Skip to content

perf(deploy): sector-selective write-flash on verify mismatch (#67)#71

Merged
zackees merged 2 commits intomainfrom
perf/issue-67-selective-write-flash
Apr 17, 2026
Merged

perf(deploy): sector-selective write-flash on verify mismatch (#67)#71
zackees merged 2 commits intomainfrom
perf/issue-67-selective-write-flash

Conversation

@zackees
Copy link
Copy Markdown
Member

@zackees zackees commented Apr 17, 2026

Summary

Closes #67.

When esptool verify-flash reports a mismatch, parse its per-region output and write only the regions that actually differ. For the common case of "only firmware changed", this skips the ~1s bootloader + partitions rewrite and avoids unnecessary flash wear.

Changes

  • parse_verify_regions() maps esptool's Verifying ... at 0x{addr} / Verification successful|failed output to per-region FlashRegion verdicts by matching the address against the deployer's three known offsets. Unknown offsets and unparseable output fall back to an empty result, which the daemon treats as "flash everything".
  • VerifyOutcome::Mismatch carries a regions: Vec<RegionVerifyResult> so the daemon can decide between full and selective writes.
  • Esp32Deployer::deploy_regions() and build_write_flash_args() issue a write_flash call that includes only the requested offset/file pairs.
  • The daemon operations handler calls deploy_regions() when parsing identified fewer than three mismatched regions; otherwise it falls through to the existing full-flash path.

Test plan

  • Unit tests cover parser success, unknown format, unknown offsets
  • Unit tests cover selective argv builder (firmware-only, all-regions default)
  • Unit tests cover empty slice rejection
  • cargo clippy --workspace --all-targets -- -D warnings clean
  • cargo fmt --all -- --check clean
  • Real-hardware validation (existing try_verify_deployment_real_esp32* tests remain ignored by default; maintainer should run one with an attached board when convenient)

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • New Features
    • Selective region flashing for ESP32 devices—when verification detects mismatches in fewer than three regions, only those specific regions are re-flashed instead of the entire firmware, significantly reducing deployment time.
    • Enhanced verification feedback providing per-region results, indicating which components (bootloader, partitions, or firmware) matched or experienced verification failures.

Closes #67.

When esptool verify-flash reports a mismatch, parse its per-region output
and write only the regions that actually differ. For the common case of
"only firmware changed", this skips the ~1s bootloader + partitions
rewrite and avoids unnecessary flash wear.

Changes:
- `parse_verify_regions()` maps esptool's `Verifying ... at 0x{addr}` /
  `Verification successful|failed` output to per-region `FlashRegion`
  verdicts by matching the address against the deployer's three known
  offsets. Unknown offsets and unparseable output fall back to an empty
  result, which the daemon treats as "flash everything".
- `VerifyOutcome::Mismatch` carries a `regions: Vec<RegionVerifyResult>`
  so the daemon can decide between full and selective writes.
- `Esp32Deployer::deploy_regions()` and `build_write_flash_args()`
  issue a `write_flash` call that includes only the requested
  offset/file pairs.
- The daemon operations handler calls `deploy_regions()` when parsing
  identified fewer than three mismatched regions; otherwise it falls
  through to the existing full-flash path.

Tests cover the parser (success, unknown format, unknown offsets) and
the selective argv builder (firmware-only, all-regions default, empty
slice rejection). The real-hardware verify tests in
`try_verify_deployment_real_esp32*` remain ignored by default.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 17, 2026

Warning

Rate limit exceeded

@zackees has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 47 minutes and 47 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 47 minutes and 47 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f3d228f4-a47c-47b2-baa4-eba0e240de97

📥 Commits

Reviewing files that changed from the base of the PR and between c1a8214 and 0ce560d.

📒 Files selected for processing (1)
  • crates/fbuild-deploy/src/esp32.rs
📝 Walkthrough

Walkthrough

The changes implement sector-selective write-flash on ESP32 deployment verification mismatch. When verify-flash reports a mismatch, per-region diff information is now parsed and extracted. If fewer than 3 regions differ, only those mismatched regions are re-flashed; otherwise, the system falls back to full-flash. This optimization eliminates redundant writes of unchanged regions like bootloader or partitions.

Changes

Cohort / File(s) Summary
Region Verification Parsing
crates/fbuild-deploy/src/esp32.rs
Added FlashRegion enum and RegionVerifyResult struct to model per-region verification outcomes. Introduced parse_verify_regions() function to extract region-level match/mismatch status from esptool verify-flash output. Extended VerifyOutcome::Mismatch variant to include parsed regions field.
Selective Write-Flash Implementation
crates/fbuild-deploy/src/esp32.rs
Added build_write_flash_args() method to construct write-flash arguments for an optional subset of regions. Added deploy_regions() method to execute selective flashing. Updated Deployer trait implementation to use build_write_flash_args() with default all-regions behavior. Includes unit tests for region parsing and selective argv generation.
Selective Region Deployment Handler
crates/fbuild-daemon/src/handlers/operations.rs
Modified deploy handler to capture region-level mismatch information from verification. When try_verify_deployment reports Mismatch, extracts regions where matched is false; if fewer than 3 regions differ and parsing succeeds, invokes deploy_regions() for selective flashing instead of full-flash fallback.

Sequence Diagram(s)

sequenceDiagram
    participant Handler as Daemon Handler
    participant Device as ESP32 Device
    participant Deployer as Esp32Deployer
    
    Handler->>Deployer: deploy(firmware_path, port)
    Deployer->>Device: esptool verify-flash (all regions)
    Device-->>Deployer: stdout/stderr with region results
    
    Deployer->>Deployer: parse_verify_regions(stdout)
    Deployer-->>Handler: VerifyOutcome::Mismatch {regions: Vec<RegionVerifyResult>}
    
    Handler->>Handler: Extract mismatched regions
    
    alt Fewer than 3 regions differ
        Handler->>Deployer: deploy_regions(firmware_path, port, mismatched_regions)
        Deployer->>Deployer: build_write_flash_args (selective regions)
        Deployer->>Device: esptool write-flash (selective offsets)
        Device-->>Deployer: success/stderr
    else 3+ regions or parse failed
        Handler->>Deployer: deploy(firmware_path, port)
        Deployer->>Device: esptool write-flash (all regions)
        Device-->>Deployer: success/stderr
    end
    
    Deployer-->>Handler: DeploymentResult
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

The changes introduce new public data structures (FlashRegion, RegionVerifyResult), a parsing function with output-dependent fallback behavior, and dual deployment pathways (selective vs. full-flash). The logic spans two files with interconnected control flow, requiring careful verification of the parsing logic, region filtering heuristics (max 3 regions threshold), and proper fallback semantics.

Possibly related issues

Poem

🐰 Selective flashing hops ahead,
No more rewriting what's not fled,
Parse each region, find what's wrong,
Flash the few, not the throng!
Bootloader rests while firmware's done. 🚀

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main performance optimization: selective region flashing when verify detects mismatches, matching the primary change throughout the codebase.
Linked Issues check ✅ Passed All primary objectives from issue #67 are addressed: parsing verify-flash output for per-region results, adding deploy_regions API, implementing selective re-flashing logic, and maintaining fallback behavior.
Out of Scope Changes check ✅ Passed All changes are directly related to issue #67 objectives; no out-of-scope modifications detected. Changes are focused on selective region flashing implementation.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch perf/issue-67-selective-write-flash

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/fbuild-deploy/src/esp32.rs`:
- Around line 817-832: deploy_regions currently can be asked to deploy specific
FlashRegion(s) but will silently drop Bootloader/Partitions when their files
(bootloader_path, partitions_path) are missing; update deploy_regions to
validate requested non-Firmware regions before calling build_write_flash_args
and return an explicit Err describing which region file is missing (reference
FlashRegion::Bootloader and FlashRegion::Partitions), leaving
build_write_flash_args unchanged (argv-only). Ensure the check iterates the
incoming regions slice (or matches the Some(&[...]) case), tests path.exists()
for the corresponding bootloader_path/partitions_path, and returns a clear error
(including the missing region name) instead of letting esptool receive no region
args.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: da4bd90c-50ff-4954-a50b-12a90316d580

📥 Commits

Reviewing files that changed from the base of the PR and between 360df2b and c1a8214.

⛔ Files ignored due to path filters (2)
  • Cargo.lock is excluded by !**/*.lock
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (2)
  • crates/fbuild-daemon/src/handlers/operations.rs
  • crates/fbuild-deploy/src/esp32.rs

Comment thread crates/fbuild-deploy/src/esp32.rs
Address CodeRabbit review on #71. If a caller passed Some(&[Bootloader])
but bootloader.bin wasn't on disk, build_write_flash_args silently
emitted zero offset/file pairs and esptool failed with an opaque usage
error. We now verify each requested region's file exists up front and
return a clear error naming the missing file.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zackees zackees merged commit accb967 into main Apr 17, 2026
76 checks passed
@zackees zackees deleted the perf/issue-67-selective-write-flash branch April 17, 2026 21:02
zackees added a commit that referenced this pull request Apr 17, 2026
…nse message (#76)

Previously `/api/deploy` always returned `message: "deploy succeeded"`
regardless of whether the deploy was a full write, a verify-skip (MD5
match), or a selective rewrite of only the differing ESP32 regions.
The distinction existed internally (from #67/#71) but was lost at the
handler layer.

Adds a `DeployOutcome` enum to `fbuild-deploy` carried on
`DeploymentResult::outcome`, populated by every deployer path:

- Esp32Deployer::deploy       -> FullFlash
- Esp32Deployer::deploy_regions -> SelectiveFlash { regions }
- AvrDeployer / TeensyDeployer -> FullFlash
- Daemon VerifyOutcome::Match short-circuit -> VerifySkip

The daemon handler composes a stable prefix
`"deploy succeeded (<describe>)"` and uses it at every response site
(plain success + four monitor-attached variants), so consumers can
distinguish a 6s MD5 skip from a 25s full write.

Covered by new unit tests in `fbuild-deploy::outcome_tests` and
`fbuild-daemon::handlers::operations::deploy_message_tests`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

perf(deploy): sector-selective write-flash on verify mismatch

1 participant